Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update CODEOWNERS: add NRL representatives #1086

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update CODEOWNERS: add NRL representatives #1086

wants to merge 1 commit into from

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Sep 17, 2024 •
edited
Loading

As previously discussed, add representatives from NRL as codeowners for ccpp-physics. I am going to create a similar PR for the ufs/dev branch in the ufs-community fork.

I will also note that there are still errors in this CODEOWNERS file (before me making any changes), as indicated by Github. I noted that the overall codeowners are the same for both forks, so therefore I only create the PR here and wait for it to make it into the ufs/dev branch (Fanglin approved this by email a couple of months ago)?

@climbfuji
Update CODEOWNERS: add NRL representatives
6c8d07a 
As previously discussed, add representatives from NRL as codeowners for ccpp-physics. I am going to create a similar PR for the ufs/dev branch in the ufs-community fork.

I will also note that there are still errors in this CODEOWNERS file (before me making any changes), as indicated by Github.
@climbfuji
Copy link
Collaborator Author

image

@climbfuji climbfuji self-assigned this Sep 17, 2024
@dustinswales
Copy link
Collaborator

image

@climbfuji I believe(?) we decided sometime ago that we weren't going to give CODEOWNERS from the ufs/dev fork write access to the NCAR:ccpp-physics repo. Hence these errors.

@climbfuji
Copy link
Collaborator Author

image

@climbfuji I believe(?) we decided sometime ago that we weren't going to give CODEOWNERS from the ufs/dev fork write access to the NCAR:ccpp-physics repo. Hence these errors.

Ok, but then should they be removed? Or will they still be assigned to review (I believe not)? Or do you just want to keep the file in sync and live with the fact that they won't be assigned as codeowners?

@dustinswales
Copy link
Collaborator

Ok, but then should they be removed? Or will they still be assigned to review (I believe not)? Or do you just want to keep the file in sync and live with the fact that they won't be assigned as codeowners?

@climbfuji You got it.
We don't want CODEOWNERS to get notified a second time for changes that go from UFS:ufs/dev -> NCAR:main, and we don't want the CODEOWNERS file to have a delta between the two repositories. So essentially we limit the "collaborators" on the NCAR side to avoid the excessive notifications, and keep the files synced. Not ideal, but we didn't have any better solutions at the time.

@climbfuji
Copy link
Collaborator Author

Ok, but then should they be removed? Or will they still be assigned to review (I believe not)? Or do you just want to keep the file in sync and live with the fact that they won't be assigned as codeowners?

@climbfuji You got it. We don't want CODEOWNERS to get notified a second time for changes that go from UFS:ufs/dev -> NCAR:main, and we don't want the CODEOWNERS file to have a delta between the two repositories. So essentially we limit the "collaborators" on the NCAR side to avoid the excessive notifications, and keep the files synced. Not ideal, but we didn't have any better solutions at the time.

Thanks, this makes sense.

@climbfuji climbfuji marked this pull request as ready for review September 17, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grantfirl grantfirl Awaiting requested review from grantfirl grantfirl is a code owner

@Qingfu-Liu Qingfu-Liu Awaiting requested review from Qingfu-Liu Qingfu-Liu is a code owner

@dustinswales dustinswales Awaiting requested review from dustinswales dustinswales is a code owner

@mzhangw mzhangw Awaiting requested review from mzhangw mzhangw is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@climbfuji climbfuji

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@climbfuji @dustinswales